Skip to content

Align pdsh benchmarks and library defaults#22399

Open
TomAugspurger wants to merge 5 commits intorapidsai:mainfrom
TomAugspurger:tom/cudf-polars-cli-alignment
Open

Align pdsh benchmarks and library defaults#22399
TomAugspurger wants to merge 5 commits intorapidsai:mainfrom
TomAugspurger:tom/cudf-polars-cli-alignment

Conversation

@TomAugspurger
Copy link
Copy Markdown
Contributor

@TomAugspurger TomAugspurger commented May 6, 2026

Description

This updates the defaults in our pdsh benchmarks to match the defaults from the library. This will make it easier to understand what values are being used based just on the command run.

@TomAugspurger
Align pdsh benchmarks and library defaults
6ec28a8 
This updates the defaults in our pdsh benchmarks to match the defaults
from the library. This will make it easier to understand what values
are being used based just on the command run.
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 6, 2026

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@github-actions github-actions Bot added Python Affects Python cuDF API. cudf-polars Issues specific to cudf-polars labels May 6, 2026
@TomAugspurger TomAugspurger added bug Something isn't working non-breaking Non-breaking change labels May 6, 2026
@GPUtester GPUtester moved this to In Progress in cuDF Python May 6, 2026
@TomAugspurger TomAugspurger marked this pull request as ready for review May 6, 2026 18:29
@TomAugspurger TomAugspurger requested a review from a team as a code owner May 6, 2026 18:29
@TomAugspurger TomAugspurger requested a review from mroeschke May 6, 2026 18:29
pentschev
pentschev approved these changes May 6, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@pentschev pentschev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks Tom.

@mroeschke mroeschke added bug Something isn't working and removed bug Something isn't working labels May 6, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026 •
edited
Loading

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 87882c4c-20d3-4e42-b0b9-650d0e25e4d5

📥 Commits

Reviewing files that changed from the base of the PR and between f7bbb0b and c20f8d3.

📒 Files selected for processing (1)
  • python/cudf_polars/cudf_polars/experimental/rapidsmpf/frontend/options.py
✅ Files skipped from review due to trivial changes (1)
  • python/cudf_polars/cudf_polars/experimental/rapidsmpf/frontend/options.py
📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Updated CLI defaults: --max-io-threads now defaults to 4; --native-parquet now defaults to False.
    • Corrected CLI help text to point to the appropriate configuration mappings for related options.
    • Fixed CLI help to report the built-in default for --num-py-executors as 8.

Walkthrough

CLI argument defaults and help text are updated across benchmark and streaming configuration files to reflect actual runtime behavior. The --max-io-threads default increases from 2 to 4, --native-parquet defaults to False instead of True, and --num-py-executors help text corrects the documented default from 1 to 8.

Changes

CLI Configuration Updates

Layer / File(s) Summary
CLI argument defaults and documentation
python/cudf_polars/cudf_polars/experimental/benchmarks/utils_new_frontends.py, python/cudf_polars/cudf_polars/experimental/rapidsmpf/frontend/options.py		 Default values and help text updated for benchmark and streaming configuration arguments: --max-io-threads default increased to 4, --native-parquet default set to False, and --num-py-executors help text corrected to reflect actual default of 8.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Align pdsh benchmarks and library defaults' clearly summarizes the main change: updating benchmark default values to match library defaults for consistency.
Description check ✅ Passed The description is directly related to the changeset, explaining that defaults in pdsh benchmarks are being updated to match library defaults to improve clarity.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai Bot reviewed May 8, 2026
View reviewed changes
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@python/cudf_polars/cudf_polars/experimental/rapidsmpf/frontend/options.py`:
- Around line 644-646: The help text for the --num-py-executors option
incorrectly names the env var as CUDF_POLARS__NUM_PY_EXECUTORS; update that help
string to the actual env var used at runtime,
CUDF_POLARS__EXECUTOR__NUM_PY_EXECUTORS. Locate the option definition for
--num-py-executors in options.py (the field wired to
CUDF_POLARS__EXECUTOR__NUM_PY_EXECUTORS) and replace the incorrect env var
reference in its help/description so the documentation matches the configured
environment variable.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e862b61a-eafd-41bc-98f1-cedbe61d240b

📥 Commits

Reviewing files that changed from the base of the PR and between 7d84936 and f7bbb0b.

📒 Files selected for processing (2)
  • python/cudf_polars/cudf_polars/experimental/benchmarks/utils_new_frontends.py
  • python/cudf_polars/cudf_polars/experimental/rapidsmpf/frontend/options.py

Comment thread python/cudf_polars/cudf_polars/experimental/rapidsmpf/frontend/options.py
@TomAugspurger
Copy link
Copy Markdown
Contributor Author

TomAugspurger commented May 9, 2026

/merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@madsbk madsbk madsbk approved these changes

@pentschev pentschev pentschev approved these changes

@Matt711 Matt711 Matt711 approved these changes

@mroeschke mroeschke Awaiting requested review from mroeschke mroeschke is a code owner automatically assigned from rapidsai/cudf-polars-codeowners

Assignees

@TomAugspurger TomAugspurger

Labels

bug Something isn't working cudf-polars Issues specific to cudf-polars non-breaking Non-breaking change Python Affects Python cuDF API.

Projects

cuDF Python
Status: In Progress

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@TomAugspurger @madsbk @pentschev @Matt711 @mroeschke @GPUtester